To fit the pattern of the rest of the system, this would be drivers/SingleWireOutput.h - you've put the ITM bits in the HAL, and this is the corresponding C++ wrapper. So it goes with SPI and friends.

I was hoping to eventually put it into mbed_retarget.cpp like you did with the serial class: https://github.com/kjbracey-arm/mbed-os/blob/d7e42d1f69e5a8f06441742d910d795c8ae22c85/platform/mbed_retarget.cpp#L128-L180

so that we can enable SWO from the config system.

My intention with this PR is to start some discussion about what would be right approach. @0xc0170 @bulislaw any comments?

I'm slightly wary of setting up a sort of "central registry" pattern where every potential device has to be plopped in there.

In #5558 there's a similar concept - the JSON choice of "default onboard network stack". You set nsapi.default-stack to either "LWIP" or "NANOSTACK". There's no central logic for that, but each of the two stacks has a code snippet that looks like

#define NANOSTACK 0x82917429
#if MBED_CONF_NSAPI_DEFAULT_STACK == NANOSTACK
OnboardNetworkStack &OnboardNetworkStack::get_default_instance() {
    return Nanostack::get_instance();
}
#endif

So only one of them will define that function.

Maybe the same concept would work here. JSON option in central place, but any target or library one can add a new selectable default without touching the core code - just invent a name.

Anyway, regardless of the "default console" selection, this needs to be exposed as a separate standalone device manually accessible as SingleWireOutput.

See my 18 Dec comments on #5356, where they're concentrating on alternative console, but I'm urging them to make sure that the device is always manually useable by the app without being forced to make it stdin/stdout.

The built-in FileHandles in my mbed_retarget.cpp are purely to preserve legacy behaviour. If anything, they will be moved out (@geky has suggested making Sink public, and discussions with @sg- have led to concensus that RawSerial should be a FileHandle, thus it can do the job of DirectSerial).

No problem, I'll move/rename it to mbed-os/drivers.

I'm fine with either invocation as well, but I think we should have it documented exactly how we expect people to do it!

I think it's better to just return 0 (indicating "end of file") or an error code (not sure which - -EBADF = "not valid file descriptor open for reading"?; -ENXIO = "... request was outside the capabilities of the device"?). Don't know whether EOF or error is better.

My dummy device implementation inside the retarget code is only reading endless NUL bytes to preserve the legacy behaviour of stdin with DEVICE_SERIAL unset. I don't think that's actually useful, or what you should do for something new.

Did a bit of background reading to compare other systems. This is an interesting general question.

/dev/null returns EOF if opened for input. But that's more likely to be used manually as a redirect for processes that expect a redirect from file, and hence understand/expect EOF on stdin.

Many real devices probably would do ENXIO on the attempt to open for read. I guess that means attempts to use a read-only device as process input would fail the read open and then you'd get EBADF later.

Linux gives EBADF if you read from a file that's open for output only.

When you nohup and detach a process in Linux it does seem you get EBADF on input (and you can change that to EOF if you redirect from /dev/null). POSIX says behaviour of nohup input is up to the implementation - it might leave input from original terminal, or get it from a file.

On balance, I guess I'll say EBADF is what to do on read. This is a "standalone" FileHandle where it's automatically opened on construction, so we should assume that the implicit open is write-only, thus give EBADF on read attempts.

If there was a device filing system, so we were able to participate in the open call, we'd return ENXIO on the attempt to open for read.

Thank you! I'll change it to EBADF.

Lesson learned, looks like I have some filesystems to update...

Ah, normal filesystems might be a bit different. Eg EACCES for access violation rather than ENXIO. I was only really thinking about devices, and particularly considering this odd case of "opened by construction" objects.

I just spend too much time Googling "POSIX open" or "POSIX read" to check POSIX specs for my own good...

Well, in the open group docs for write:
http://pubs.opengroup.org/onlinepubs/9699919799/functions/write.html

[EBADF]
The fildes argument is not a valid file descriptor open for writing.

Pretty straightforward for POSIX docs actually. 😆

A quick test program confirms this is what Linux returns.

Fortunately reading/writing to a file after opening it in a specific mode is pretty uncommon outside of user errors. I'd be worried if someone was relying on a specific error code in that situation.

This isn't in itm_api.h? Skipping a layer?

ITM_SendChar is defined in:

cmsis/TARGET_CORTEX_M/core_cm3.h
cmsis/TARGET_CORTEX_M/core_cm4.h
etc.

I'm not sure how to explicitly include the right header file based on core.

ah, #include "cmsis.h" should do the trick.

yes, cmsis should be used as a generic cmsis header file

I think you'll need to include platform/FileHandle.h.

Thank you! I'll add the missing include.

I know it's a dummy function but could we get some description? What should it initialise (channel 0?). Here's some example https://github.com/ARMmbed/mbed-os/blob/feature-hal-spec-rtc/hal/rtc_api.h also can you add it to doxy please.
@c1728p9 any comments?

Not a single comment :P @SenRamakri please review

Seemed a bit premature until we've all agreed on the functionality! 😄

@marcuschangarm - Since this is coming into master its better to have some comments/documentation on what this class is?

@marcuschangarm - Since this is coming into master its better to have some comments/documentation on what this class is?

Potentially infinite loop, can we please break this loop and return with an error after waiting enough? I don't how much is "enough", you can define that.

All Silicon Labs clock initialization code use this kind of infinite loop. Changing this single loop with an arbitrary loop counter would break the pattern.

It would be better if we can #define these magic values.

Magic value 0x400003FF and others below. Can we please define them with corresponding fields? Also it looks like, we are writing to upper nibble which is read-only I guess. We are also enabling CYCCNTENA, POSTINIT etc, are these required? May be we should review this value as to what all we need to enable.